Fix: Surface destructive schema changes on forward-only models during dev plans (#5719)#5732
Fix: Surface destructive schema changes on forward-only models during dev plans (#5719)#5732brucearctor wants to merge 2 commits into
Conversation
… dev plans (SQLMesh#5719) Extend _check_destructive_additive_changes in PlanBuilder to also inspect indirectly modified forward-only snapshots for destructive/additive schema changes. Previously, only directly modified snapshots were checked, and the evaluator-level check (MigrateSchemasStage) is skipped for dev plans. Changes: - Refactor schema diffing into reusable _check_schema_change helper - Pass indirectly_modified mapping to _check_destructive_additive_changes - Add loop to check indirectly modified forward-only snapshots - Add 5 new tests for is_dev=True scenarios (ERROR/WARN/ALLOW modes, indirect modification, --forward-only flag)
|
@brucearctor it looks like the direct-modification case already raises PlanError on current main. Could you maybe clarify what version/scenario you reproduced, and update the PR description to state that the net-new behavior is the indirect-modification loop (plus the _check_schema_change refactor). |
|
This PR was opened months ago. Way too long has passed for me to recall. Too stale for me to worry about - it was solving someone else's reported bug -- i was trying to get involved with supporting this project. |
|
Closing, as i thats too long to maintain context |
|
@brucearctor Apologize we didn't pick it up sooner. The TSC was formed and starting to be active in the last couple of weeks after sqlmesh had been donated to Linux Foundation. We'll be more active going forward though |
|
All good. Apologies not needed. I just cant recall the status as was someone else's issue. And given 3 months has passed since opening that PR, it seemed reasonable for me to consider sufficiently stale. |
2 participants
Summary
Fixes #5719. Destructive schema changes on forward-only models are now surfaced at plan-build time during dev plans (
sqlmesh plan dev), not just prod plans.Problem
When a forward-only model has columns removed or types narrowed,
sqlmesh plan devand PR checks would succeed, but the production deployment would fail with aDestructiveChangeErrorduringMigrateSchemasStage. This left the production environment in an unfinalized state.Root Cause
PlanBuilder._check_destructive_additive_changesalready checked for destructive schema changes at plan-build time (regardless ofis_dev), but only for directly modified snapshots. Indirectly modified forward-only models (e.g., downstream models inheriting schema changes from an upstream dependency) were not checked. The evaluator-level check inMigrateSchemasStagecatches these for prod, but that stage is skipped for dev plans.Changes
sqlmesh/core/plan/builder.py_check_schema_changehelper methodindirectly_modifiedmapping to_check_destructive_additive_changestests/core/test_plan.pyAdd 5 new tests for
is_dev=Truescenarios:test_forward_only_destructive_change_dev_plan— ERROR mode raisesPlanErrortest_forward_only_destructive_change_dev_plan_warn— WARN mode logs warningtest_forward_only_destructive_change_dev_plan_allow— ALLOW mode passes silentlytest_forward_only_indirect_destructive_change_dev_plan— indirect modification path exercisedtest_forward_only_flag_destructive_change_dev_plan—--forward-onlyflag with dev planTest Results
All 27 forward-only/destructive tests pass (5 new + 22 existing), zero regressions.